Skip to content

[build] avoid use generator expression to add linker parameter#29636

Merged
edymtt merged 2 commits intoswiftlang:masterfrom
edymtt:address-generator-linker-incompatibility
Feb 5, 2020
Merged

[build] avoid use generator expression to add linker parameter#29636
edymtt merged 2 commits intoswiftlang:masterfrom
edymtt:address-generator-linker-incompatibility

Conversation

@edymtt
Copy link
Contributor

@edymtt edymtt commented Feb 4, 2020

Generator expressions seem not to allow the LINKER: prefix to be
expanded correctly when used in target_link_options.

To solve this, undo the change from #29451 and instead
use an if statement to add the flag when needed.

Addresses rdar://problem/59117166

@edymtt
[build] avoid use generator expression to add linker parameter
c60c69c 
Generator expressions seem not to allow the `LINKER:` prefix to be
expanded correctly when used in `target_link_options`.

To solve this, undo the change from swiftlang#29451 and instead
use an `if` statement to add the flag when needed.

Addresses rdar://problem/59117166
@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

@swift-ci please test

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

@swift-ci please test Windows platform

@swift-ci
Copy link
Contributor

swift-ci commented Feb 4, 2020

Build failed
Swift Test OS X Platform
Git Sha - c60c69c

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

The failure on Swift Test OS X Platform looks related to machine configuration

******************** TEST 'Swift(macosx-x86_64) :: Python/python_lint.swift' FAILED ********************
Script:
--
: 'RUN: at line 8';   /usr/bin/python /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/swift/utils/python_lint.py
--
Exit Code: 1

Command Output (stdout):
--

The flake8 and flake8-import-order Python packages are required for linting,
but these were not found on your system.

You can install these using:

    python -m pip install flake8
    python -m pip install flake8-import-order

For more help, see http://flake8.pycqa.org.


--

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

@swift-ci please test macOS

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

@swift-ci please smoke-test

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

@swift-ci please smoke test

@dan-zheng dan-zheng mentioned this pull request Feb 4, 2020
Merged
@compnerd
Copy link
Member

compnerd commented Feb 4, 2020

It is supported, but I think that it could be a limitation of 3.15. Could you please update the comment or better yet, add it as:

  if(CMAKE_VERSION VERSION_LESS 3.16)
    ...
  else()
    ...
  endif()

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

@swift-ci please test

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

@swift-ci please smoke test

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

@swift-ci please test Windows platform

@swift-ci
Copy link
Contributor

swift-ci commented Feb 4, 2020

Build failed
Swift Test Linux Platform
Git Sha - c60c69c

@edymtt
Copy link
Contributor Author

edymtt commented Feb 4, 2020

Committed e438ce7 with the proposal -- we will introduce a bit of code duplication, but (a) both code paths are next to each other and (b) we will not forget about the opportunity to make this code more streamlined

@swift-ci
Copy link
Contributor

swift-ci commented Feb 4, 2020

Build failed
Swift Test OS X Platform
Git Sha - c60c69c

@edymtt
Copy link
Contributor Author

edymtt commented Feb 5, 2020

@swift-ci please test Windows platform

compnerd
compnerd approved these changes Feb 5, 2020
View reviewed changes
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to squash these before merging

@edymtt edymtt merged commit 8a76358 into swiftlang:master Feb 5, 2020
edymtt added a commit to edymtt/swift that referenced this pull request Mar 10, 2020
@edymtt
[build] set linker parameters in a single way
77a6217 
After noticing that also in CMake 3.16 the LINKER: prefix is not
expanded correctly when used in `target_link_options`, prefer to set the
linker parameters in a more verbose way and leave a comment behind on
when this behavior was observed in case we want to change the
implementation later.

Follow up to swiftlang#29636.
Addresses rdar://problem/59732421
@edymtt edymtt mentioned this pull request Mar 10, 2020
Merged
edymtt added a commit that referenced this pull request Mar 11, 2020
@edymtt
[build] set linker parameters in a single way (#30339)
b3e8661 
After noticing that also in CMake 3.16 the LINKER: prefix is not
expanded correctly when used in `target_link_options`, prefer to set the
linker parameters in a more verbose way and leave a comment behind on
when this behavior was observed in case we want to change the
implementation later.

Follow up to #29636.
Addresses rdar://problem/59732421
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@compnerd compnerd compnerd approved these changes

@shahmishal shahmishal Awaiting requested review from shahmishal

@clackary clackary Awaiting requested review from clackary

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@edymtt @swift-ci @compnerd